Skip to content

[https://nvbugs/6507081][fix] Refresh the fakes only — add reasoning_parser = None to…#16841

Open
trtllm-agent wants to merge 2 commits into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6507081
Open

[https://nvbugs/6507081][fix] Refresh the fakes only — add reasoning_parser = None to…#16841
trtllm-agent wants to merge 2 commits into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6507081

Conversation

@trtllm-agent

@trtllm-agent trtllm-agent commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: Test's _FakeCompletionGeneratorArgs lacks reasoning_parser and the fake server lacks _input_proc_executor, both of which the real openai_completion path reads (openai_server.py:1923, 1955); the resulting AttributeError is caught and returned as a JSONResponse (no body_iterator).
  • Fix: Refresh the fakes only — add reasoning_parser = None to _FakeCompletionGeneratorArgs (safe: add_thinking_budget_logits_processor short-circuits when thinking_token_budget is None), and set server._input_proc_executor = None (safe: loop.run_in_executor(None, ...) uses the default executor).
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Dev Engineer Review

  • Updated tests/unittest/llmapi/test_llm.py OpenAI prompt-streaming reuse test scaffolding to better match the real server interface by adding missing fake attributes:
    • Set _FakeCompletionGeneratorArgs.reasoning_parser = None.
    • Set server._input_proc_executor = None.
  • These changes are minimal and intended to prevent AttributeError failures from test doubles that didn’t expose fields expected by the openai_completion path.
  • Updated tests/integration/test_lists/waives.txt to remove the waiver entry for unittest/llmapi/test_llm.py::test_openai_completion_list_prompt_stream_reuses_stream_metadata, re-enabling the test in integration runs.

QA Engineer Review

  • Modified test scaffolding in tests/unittest/llmapi/test_llm.py:
    • test_openai_completion_list_prompt_stream_reuses_stream_metadata: adjusted mocked server/fake objects to include reasoning_parser and _input_proc_executor attributes required by the code under test.
    • No test functions were added or removed; behavior changes are limited to test double compatibility.
  • Integration test-list coverage:
    • The affected test is now un-avoided/unwaived via tests/integration/test_lists/waives.txt.
  • Modified test-list file:
    • tests/integration/test_lists/waives.txt: removed waiver for unittest/llmapi/test_llm.py::test_openai_completion_list_prompt_stream_reuses_stream_metadata.
  • Verdict: sufficient.

…erver surface

Signed-off-by: handongl <handongl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 50cbdb4e-86b1-4b5a-964d-8299dfe2de96

📥 Commits

Reviewing files that changed from the base of the PR and between 424ca96 and 2a7e357.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

Walkthrough

The OpenAI prompt-streaming test scaffolding now includes missing reasoning_parser and _input_proc_executor attributes, and its integration test waiver has been removed.

Changes

Streaming test mock alignment

Layer / File(s) Summary
Update streaming test mocks and waiver
tests/unittest/llmapi/test_llm.py, tests/integration/test_lists/waives.txt
Adds the required fake generator and server attributes, and removes the skip entry for the streaming metadata reuse test.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested reviewers: qijune, tongyuantongyu, emmaqiaoch, asfiyab-nvidia, stanleysun639

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific and clearly matches the main change to the test fakes.
Description check ✅ Passed The description explains the root cause, fix, test plan, and bug link, so it is mostly complete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@2ez4bz

2ez4bz commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

/bot run

@2ez4bz
2ez4bz enabled auto-merge (squash) July 24, 2026 16:43
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #61605 [ run ] triggered by Bot. Commit: 424ca96 Link to invocation

Signed-off-by: William Zhang <133824995+2ez4bz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants